[SAC-27535] [tap-frontapp] - fix extraction failures #28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RushiT0122
changed the base branch from
master
to
feature/SAC-27536-frontapp-libraries
romilqlik
force-pushed
the
feature/SAC-27536-frontapp-libraries
branch
from
976e331 to
9f6165a
Compare
romilqlik
force-pushed
the
feature/SAC-27535-frontapp
branch
from
5b4cdab to
065489b
Compare
rdeshmukh15
reviewed
May 22, 2025
rdeshmukh15
reviewed
May 22, 2025
Comment on lines
-18
to
-26
| #def check_authorization(atx): | ||
| # atx.client.get('/settings') | ||
|
|
||
|
|
||
| # Some taps do discovery dynamically where the catalog is read in from a | ||
| # call to the api but with the odd frontapp structure, we won't do that | ||
| # here we never use atx in here since the schema is from file but we | ||
| # would use it if we pulled schema from the API def discover(atx): | ||
|
|
Contributor
There was a problem hiding this comment.
Any reason this is removed?!
Contributor
Author
There was a problem hiding this comment.
This is removed because this block was originally part of the older inlines discover() in init.py.
Since the discovery logic has now been moved to discover.py, the comments has been cleaned up.
rdeshmukh15
reviewed
May 22, 2025
tap_frontapp/__init__.py
Outdated
| from singer import utils | ||
| from singer.catalog import Catalog, CatalogEntry, Schema | ||
| from singer.catalog import Catalog | ||
| from . import streams |
Contributor
There was a problem hiding this comment.
This import from . import streams is not getting used anywhere in the file
rdeshmukh15
reviewed
May 22, 2025
tap_frontapp/sync.py
Outdated
| @@ -0,0 +1,38 @@ | |||
| """Module for syncing selected FrontApp streams using Singer framework.""" | |||
Contributor
There was a problem hiding this comment.
No need of file level docstrings. Can you please remove wherever added.
Removed type annotations from the file as per review
Removed type annotations from file as per review
Removed type annotations as per review
Removed type annotations as per comment
Removed unnecessary header as per review
Removed header comment
Removed file level docstring
romilqlik
commented
May 22, 2025
Comment on lines
-18
to
-26
| #def check_authorization(atx): | ||
| # atx.client.get('/settings') | ||
|
|
||
|
|
||
| # Some taps do discovery dynamically where the catalog is read in from a | ||
| # call to the api but with the odd frontapp structure, we won't do that | ||
| # here we never use atx in here since the schema is from file but we | ||
| # would use it if we pulled schema from the API def discover(atx): | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This is removed because this block was originally part of the older inlines discover() in init.py.
Since the discovery logic has now been moved to discover.py, the comments has been cleaned up.
Add EOF
rdeshmukh15
approved these changes
May 22, 2025
rdeshmukh15
merged commit 62f087f
into
feature/SAC-27536-frontapp-libraries
3 checks passed
rdeshmukh15
added a commit
that referenced
this pull request
Nov 24, 2025
* [SAC-27536] Frontapp App libary changes * [SAC-27535] [tap-frontapp] - fix extraction failures (#28) * [SAC-27536] Refactored discovery and sync- logic * [SAC-27535] Final clean up: discovery fixes,init and sync * Update discover.py Removed type annotations from the file as per review * Update schemas.py Removed type annotations from file as per review * Update sync.py Removed type annotations as per review * Update sync.py * Update __init__.py Removed type annotations as per comment * Update __init__.py * Update __init__.py Removed unnecessary header as per review * Update __init__.py * Update __init__.py Removed header comment * Update sync.py Removed file level docstring * Update test_client.py Add EOF --------- Co-authored-by: gl-romil <[email protected]> * Update discover.py Added LOGGER.info * Update __init__.py Credentials Validation * Update setup.py * Update __init__.py * Update discover.py Moved validate_credentials * Update __init__.py * Update __init__.py * Update discover.py * Update test_client.py * Qa bugfixes from sac 27536 (#30) * Remove non-discoverable inclusion-reason metadata- as per QA feedback * Add JSON validator step to Circleci as per QA feedback * Remove selected by-default metadata as per feedback --------- Co-authored-by: gl-romil <[email protected]> * requests update to 2.32.4 * selected field removal from schema files (#33) * [SAC-27953] - Rate limit change for `get_report_metrics` API (#34) * rate limit change * rate limit change * [SAC-27953] - Usage of `retry-after` header for 429 errors and throttling for `create_report` API (#35) * RETRY_RATE_LIMIT change * period set to 3 * Update config.yml --------- Co-authored-by: gl-romil <[email protected]> Co-authored-by: “rdeshmukh15” <[email protected]> Co-authored-by: Tushar Mittal <[email protected]> Co-authored-by: Rutuja Deshmukh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
Enhanced http.py to handle rate-limiting using back off strategy.
Added support for X-Ratelimit-Remaining and X-Ratelimit-Reset headers.
Manual QA steps
Ran unit tests:
test_client.py
Circle CI passes.
Risks
Any dependency might affect singer execution.
Rollback steps
AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code